Sta upstream 10/13 by dsengupta0628 · Pull Request #422 · The-OpenROAD-Project/OpenSTA

dsengupta0628 · 2026-10-06T13:30:08Z

Latest STA from 10/13 morning upstream

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

…(#521) * fix for issue 389 Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * use libs from test/ dir not examples/ Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * Remove port_direction_reinit test initSta() is intended to be called once per process; the test exercised unsupported re-initialization. * Remove PortDirection::init/destroy Singletons are static objects; nothing to allocate or free. --------- Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

* Report the tcl traceback for an error while reading a file include_file reads a file one complete command at a time and evaluates each with catch, so the errorInfo tcl builds for a command that fails is only reachable there. It is discarded, and the error is reported as a single line naming the file and line but not the procs that raised it. sta_error_traceback, 0 by default, keeps it. catch is given its options dict and -errorinfo is used in place of the bare result; since errorInfo begins with the error message, the existing check that prepends the file and line only once is unaffected, and both the sta_continue_on_error report and the raise at the end of the file pick it up. The uplevel frames include_file itself adds are trimmed from the end so the traceback ends at the command read from the file. With the variable at 0 the reported text is unchanged. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * Report the start line of an incomplete command Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * Report the start line of a multi-line command that failed include_line counts lines as they are read, so by the time a command spanning several lines runs it names the command's last line. An error inside an if that opens on line 3 and closes on line 7 was reported as line 7, and a warning from a command continued across lines 14 to 18 as line 18. cmd_start_line holds the first line of the command being accumulated. include_file reports it, and sdc_file_line returns it so the messages raised with a msg id while reading a file use it as well. Single line commands are unaffected. write_path_spice_arc_sense.ok is the one golden whose warning moves, from the last line of the command to the line the command starts on. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * Report a multi-line command error at its first line When reading stops at the first error, include_file built the message with include_line, the last line of the failing command. Use cmd_start_line, as the continue-on-error message and sdc_file_line do. Remove the sta_error_traceback docs and test, superseded by the $errorInfo traceback in 1b066c3. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * Clear include_error_info at the start of include_file include_file saves the failing command's traceback in include_error_info, but only include clears it. read_sdc calls include_file directly, so after a failed read_sdc the traceback was left behind and reused by a later include error with no traceback of its own (e.g. cannot open the file). Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> --------- Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

PortDirection::init/destroy were removed in f3cd9b1 (#521); the directions are now static objects that need no initialization. Drop the stale init() calls and the null-check guards around them. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

MultiDrvrNet::dcalcDrvr/setDcalcDrvr were removed in bdd66c5 ("dcalc multi-drvr do not use levels") and replaced by loadSlewsInited/setLoadSlewsInited. Retarget the accessor tests to the new API. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Sta::findDelays() now passes Graph::vertex_level_max instead of levelize_->maxLevel() (e3326b0), so the delay_calc debug message reports level 16777215. Timing results are unchanged. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

gemini-code-assist

Code Review

This pull request refactors several core components of the timing analysis engine, including replacing dynamic PortDirection singletons with static constexpr instances, optimizing levelization with lazy initialization, adding port limit calculations to timing models, and improving Tcl error tracebacks. However, several critical issues were identified in the review: a performance bottleneck in findCrpr due to heap allocations and O(N log N) complexity from std::map, a typo in Levelize::findBackEdges that iterates over roots_ instead of the sorted roots sequence (introducing non-determinism), a bug in zeroSlewAndWireDelays where passing true for merge prevents proper zeroing, and variable shadowing of pin in setPortLimits.

gemini-code-assist · 2026-10-06T13:32:14Z

+  std::map<const Pin*, const Path*> src_pin_map;
+  const Path *src = src_clk_path1;
+  while (src && !src->isNull()) {
+    src_pin_map[src->pin(this)]  = src;
+    src = src->prevPath();
+  }
+
+  const Path *tgt = tgt_clk_path1;
+  src = nullptr;
+  Pin *tgt_pin = nullptr;
+  while (tgt && !tgt->isNull()) {
+    tgt_pin = tgt->pin(this);
+    auto src_itr = src_pin_map.find(tgt_pin);
+    if (src_itr != src_pin_map.end()) {
+      src = src_itr->second;
+      break;
    }
+    tgt = tgt->prevPath();
  }


Using std::map here introduces $O(N \log N)$ complexity and, more importantly, performs heap allocations for every node in the path. Since findCrpr is called frequently during timing analysis, this can be a significant performance bottleneck.\n\nSince the clock path is a tree and prevPath() acts as a parent pointer, finding the common pin is equivalent to finding the Lowest Common Ancestor (LCA) of two nodes in a tree. We can achieve this in $O(N)$ time and $O(1)$ auxiliary space without any heap allocations by using the standard two-pointer technique (aligning the path depths first and then walking in lockstep).

int src_len = 0; const Path *src_curr = src_clk_path1; while (src_curr && !src_curr->isNull()) { src_len++; src_curr = src_curr->prevPath(); } int tgt_len = 0; const Path *tgt_curr = tgt_clk_path1; while (tgt_curr && !tgt_curr->isNull()) { tgt_len++; tgt_curr = tgt_curr->prevPath(); } const Path *src = src_clk_path1; const Path *tgt = tgt_clk_path1; if (src_len > tgt_len) { for (int i = 0; i < src_len - tgt_len; ++i) { src = src->prevPath(); } } else if (tgt_len > src_len) { for (int i = 0; i < tgt_len - src_len; ++i) { tgt = tgt->prevPath(); } } Pin *tgt_pin = nullptr; while (src && !src->isNull() && tgt && !tgt->isNull()) { if (src->pin(this) == tgt->pin(this)) { tgt_pin = tgt->pin(this); break; } src = src->prevPath(); tgt = tgt->prevPath(); }

gemini-code-assist · 2026-10-06T13:32:14Z

+  VertexSeq roots = sortedRootsWithFanout();
+  for (Vertex *vertex : roots_) {


There is a typo here: the code initializes a local roots sequence sorted by fanout to ensure deterministic traversal, but then iterates over the member variable roots_ instead. Since roots_ is a VertexSet (which may be ordered by pointer addresses or unordered), iterating over it directly can introduce non-determinism in the DFS traversal and loop-breaking order.

VertexSeq roots = sortedRootsWithFanout(); for (Vertex *vertex : roots) {

likely typo upstream- needs fixing

gemini-code-assist · 2026-10-06T13:32:14Z

+          annotateLoadDelaySlew(drvr_vertex, wire_edge, delay_zero,
+                                load_vertex, delay_zero,
+                                true, rf, min_max, ap_index);


In zeroSlewAndWireDelays, passing true for the merge parameter to annotateLoadDelaySlew is a bug. The purpose of this function is to unconditionally reset/zero the downstream wire delays and slews (as done in the original code). If merge is true, the values will only be updated if they satisfy the delayGreater condition, which prevents them from being reset to 0.0 if they already have non-zero values. This should be false to match the original unconditional zeroing behavior.

annotateLoadDelaySlew(drvr_vertex, wire_edge, delay_zero, load_vertex, delay_zero, false, rf, min_max, ap_index);

gemini-code-assist · 2026-10-06T13:32:14Z

+    const Pin *pin = pin_iter->next();
+    const LibertyPort *port = network_->libertyPort(pin);


The loop variable pin shadows the function parameter pin. Renaming the loop variable to conn_pin avoids shadowing and improves code clarity.

const Pin *conn_pin = pin_iter->next(); const LibertyPort *port = network_->libertyPort(conn_pin);

maliberty · 2026-10-06T21:09:52Z

Have you checked for QoR impact?

dsengupta0628 · 2026-10-06T21:11:34Z

Have you checked for QoR impact?

not yet- there seems to be a correctness issue that I have raised with upstream. will wait for that before overloading CI with runs

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Upstream ed4450b ("Network::highestConnectedNet const") changed highestNetAbove/highestConnectedNet to take and return const Net*. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Upstream f861c62 ("required init resolves #523") initializes path required times to INF instead of 0.0, so paths whose required time is never computed now report required/slack as INF (1e+30). In search_network_sta_deep, reg1/CK is no longer queued for required recalculation since its required time no longer changes from 0 to INF. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

The StaDesignTest fixture only runs updateTiming(), which does not compute required times. Before upstream f861c62 uncomputed requireds were 0.0, so these tests found a path with a bogus finite slack; now they are INF and no path is selected. Call findRequireds() first. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Since upstream 859dbba ("set_max_borrow incremental"), Sta::setLatchBorrowLimit(const Pin*) looks up the pin's instance, so a nullptr pin crashes. The StaInitTest fixture has no design to supply a real pin; the pin overload is covered on a linked design by SdcDesignTest (TestSdcStaDesign.cc) and tcl.search.latch_timing. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

CLAassistant · 2026-10-12T21:46:47Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ dsengupta0628
❌ jjcherry56
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Upstream 574500d ("endpoint predicate") removed Search::searchAdj() and the SearchThru predicate behind it. These tests only checked the accessor returned non-null; evalPred() keeps its own coverage. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Tcl tests for issues found while merging upstream OpenSTA and fixed upstream. They use only existing test/ libraries (asap7_small, gf180mcu_sram) and reg1_asap7.v, plus a small hierarchical netlist and SPEF for spef_hier_reduce. - disable_timing_load_slew: load slews left at +/-INF on disabled drivers (9ee64ae; fixed 9d588bc, 99ffdff) - parasitic_network_new_net: makeParasiticNetwork on a net created after parasitics were pre-built (47f91f2; fixed 6d8f13f) - wireload_before_link: set_wire_load_model with no linked design (47f91f2; fixed 6d8f13f) - spef_hier_reduce: read_spef -path -reduce on a hierarchical port net (becb329; fixed 7b26190) - set_max_time_borrow_pg_pin: set_max_time_borrow pin/cell forms on a cell with power/ground pins (859dbba; fixed 25e089d, 6f37a1d) Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 · 2026-10-13T16:15:03Z

Upstream OpenSTA changed Network::highestNetAbove/highestConnectedNet to take and return const Net*, so OpenROAD's dbNetwork overrides and the TestDbSta.cc callers had to switch to const Net* to compile. PR here https://github.com/The-OpenROAD-Project-private/OpenROAD/pull/3922

dsengupta0628 · 2026-10-13T16:39:26Z

Secure CI http://secure-ci:8080/job/SB/job/secure-update_or_sta_1013/

jjcherry56 and others added 19 commits September 24, 2026 12:51

dcalc multi-drvr do not use levels

bdd66c5

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

crpr do not use levels

9ef4b4f

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

comment

f1237f7

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

GraphDelayCalc::findDelays()

e3326b0

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Levelize::ensureRoots()

4654362

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Levelize::ensureBackEdges()

9010436

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

GraphDelayCalc unvirutal

0eaeba0

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

write_timing_model: port max_transition/max_capacitance #518

fe1f931

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

$errorInfo traceback #516

1b066c3

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

GraphDelayCalc::findDelays bfs iter ensureSize #519

5adf472

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

untabify

8b4cc28

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

GraphDelayCalc::findDriverDelays !!

7a034b5

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Merge remote-tracking branch 'upstream/master' into sta_update_1005

3c38f11

dcalc multi driver threads

f439bc8

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

dsengupta0628 self-assigned this Oct 6, 2026

gemini-code-assist Bot reviewed Oct 6, 2026

View reviewed changes

dsengupta0628 requested review from jhkim-pii and maliberty October 6, 2026 13:44

jjcherry56 added 5 commits October 6, 2026 16:23

back out part of bdd66c5

9ee64ae

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Levelize::findBackEdges typo

a1fa634

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

dcalc multi driver nets lock

a2abda6

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

parasitics pre-build, rm lock

a54fb95

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

spef lock passes fast

949da29

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

jjcherry56 and others added 10 commits October 10, 2026 15:11

set_max_borrow incremental resolves #527

859dbba

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

required init resolves #523

f861c62

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Merge remote-tracking branch 'upstream/master' into sta_update_1005

129dafe

Search::arrivalInvalid

25e089d

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Search::requiredInvalid

6f37a1d

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Merge remote-tracking branch 'upstream/master' into sta_update_1005

b9a4053

test: use const Net* for highestNetAbove in network gtest

62be933

Upstream ed4450b ("Network::highestConnectedNet const") changed highestNetAbove/highestConnectedNet to take and return const Net*. Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 and others added 8 commits October 12, 2026 17:54

Graph skip wire edges from passive bidirect pads on driven nets (#525)

fc6ed30

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

rm return/continue

e7c4444

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

endpoint predicate

574500d

Signed-off-by: James Cherry <34749589+jjcherry56@users.noreply.github.com>

Merge remote-tracking branch 'upstream/master' into sta_update_1005

db43e7c

test: register new regression tests in regression_vars.tcl

d619d08

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

test: fail disable_timing_load_slew on INF load slews

80fbbb2

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

dsengupta0628 changed the title ~~Sta upstream 10/05~~ Sta upstream 10/13 Oct 13, 2026

dsengupta0628 marked this pull request as ready for review October 13, 2026 16:39

dsengupta0628 mentioned this pull request Oct 13, 2026

Or sta update 1012 The-OpenROAD-Project/OpenROAD#11726

Merged

5 tasks done

dsengupta0628 changed the title ~~Sta upstream 10/13~~ [DO NOT MERGE] Sta upstream 10/13 Oct 13, 2026

dsengupta0628 changed the title ~~[DO NOT MERGE] Sta upstream 10/13~~ Sta upstream 10/13 Oct 13, 2026

dsengupta0628 mentioned this pull request Oct 13, 2026

est: make OpenSTA's parasitic entries before making parasitics The-OpenROAD-Project/OpenROAD#11652

Closed

maliberty approved these changes Oct 13, 2026

View reviewed changes

maliberty merged commit 3eed955 into The-OpenROAD-Project:master Oct 13, 2026
7 of 9 checks passed

maliberty deleted the sta_update_1005 branch October 13, 2026 20:07

Navigation Menu

Repository navigation

Sta upstream 10/13 - #422

Sta upstream 10/13#422
maliberty merged 59 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:sta_update_1005

dsengupta0628 commented Oct 6, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Uh oh!

dsengupta0628 Oct 6, 2026

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Uh oh!

maliberty commented Oct 6, 2026

Uh oh!

dsengupta0628 commented Oct 6, 2026

Uh oh!

CLAassistant commented Oct 12, 2026

Uh oh!

dsengupta0628 commented Oct 13, 2026

Uh oh!

dsengupta0628 commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		VertexSeq roots = sortedRootsWithFanout();
		for (Vertex *vertex : roots_) {

		const Pin *pin = pin_iter->next();
		const LibertyPort *port = network_->libertyPort(pin);

Conversation

dsengupta0628 commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

dsengupta0628 Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty commented Oct 6, 2026

Uh oh!

dsengupta0628 commented Oct 6, 2026

Uh oh!

CLAassistant commented Oct 12, 2026

Uh oh!

dsengupta0628 commented Oct 13, 2026

Uh oh!

dsengupta0628 commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dsengupta0628 commented Oct 6, 2026 •

edited

Loading